Skip to content

Made improvements to combinations.py #3681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 26, 2020
Merged

Made improvements to combinations.py #3681

merged 4 commits into from
Oct 26, 2020

Conversation

Rolv-Apneseth
Copy link
Contributor

I have made some improvements to combinations.py.

Added:

  • A description at the top, which includes a wikipedia link
  • Type hints to the combinations function
  • Some more doctests, which include 0 and negative numbers
  • A check within the function so the function does not attempt to get factorials of negative numbers
  • Some print statements in the main block which show some practical applications of the function

If you would like anything else, or if you don't like the changes, please let me know. Thanks!

@Rolv-Apneseth
Copy link
Contributor Author

In relation to #3682

Copy link
Member

@realDuYuanChao realDuYuanChao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct!

# If either of the conditions are true, the function is being asked
# to calculate a factorial of a negative number, which is not possible
if n < k or k < 0:
print("Invalid value for n and/or k given.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should throw an exception instead of return -1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm kind of new to doctests in general, how do I make the doctest for an exception?

Copy link
Contributor Author

@Rolv-Apneseth Rolv-Apneseth Oct 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nvm think I got it

@realDuYuanChao realDuYuanChao merged commit 8f81c46 into TheAlgorithms:master Oct 26, 2020
@realDuYuanChao
Copy link
Member

@Rolv-Apneseth Thanks for your contribution :)

@Rolv-Apneseth
Copy link
Contributor Author

No problem!

stokhos pushed a commit to stokhos/Python that referenced this pull request Jan 3, 2021
* Made improvements to combinations.py

* Update maths/combinations.py

Co-authored-by: Du Yuanchao <[email protected]>

* Function now raises an error when given invalid input

* Update maths/combinations.py

Co-authored-by: Du Yuanchao <[email protected]>
peRFectBeliever pushed a commit to peRFectBeliever/Python that referenced this pull request Apr 1, 2021
* Made improvements to combinations.py

* Update maths/combinations.py

Co-authored-by: Du Yuanchao <[email protected]>

* Function now raises an error when given invalid input

* Update maths/combinations.py

Co-authored-by: Du Yuanchao <[email protected]>
Panquesito7 pushed a commit to Panquesito7/Python that referenced this pull request May 13, 2021
* Made improvements to combinations.py

* Update maths/combinations.py

Co-authored-by: Du Yuanchao <[email protected]>

* Function now raises an error when given invalid input

* Update maths/combinations.py

Co-authored-by: Du Yuanchao <[email protected]>
shermanhui pushed a commit to shermanhui/Python that referenced this pull request Oct 22, 2021
* Made improvements to combinations.py

* Update maths/combinations.py

Co-authored-by: Du Yuanchao <[email protected]>

* Function now raises an error when given invalid input

* Update maths/combinations.py

Co-authored-by: Du Yuanchao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants